Two-tier theme picker + Material 3 dark themes (Dark High Contrast, Dracula, Nord, Monokai, Ayu, Catppuccin)#57
Conversation
Replace the single color-swatch grid with a ThemePickerPreference that opens a modern named list of full-fledged Material 3 themes (preview swatch + name), with a trailing 'Classic colors...' row that opens the original swatch grid for the legacy single-accent palettes. - New ThemePickerPreference supersedes ColorPickerPreference (removed). - Theme.MODERN_THEMES declarative catalog (Dynamic for now); legacy grid skips any id present in the modern list so there's no duplication. - Persistence unchanged: same appTheme int key, same persist-then-notify ordering, so theme apply/recreate behavior is identical. No theme behavior changes; this is an isolated picker refactor.
Address CodeRabbit findings: - Use safe cast (as?) with early return for the widget_frame lookup. - Add contentDescription to modern list rows (theme name) and classic grid swatches (Color theme %d) for screen-reader support.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
ChangesTheme System Expansion
Theme Visual Definition and Layout Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
Address Greptile review: - Wrap the modern theme list in a ScrollView so it stays usable on compact screens as Theme.MODERN_THEMES grows. - Bind the preference swatch off the holder's actual view state (widgetFrame.childCount) instead of an instance flag, so a recycled/ re-inflated holder still gets its swatch. Drops the unused 'inited' field.
A full-fledged M3 dark-only theme on a true black (#000000) canvas: green-primary action accent (GitHub style) with a bright blue secondary for links/selection and high-contrast near-white text. - Theme.DARK_HIGH_CONTRAST (id 25) mapped to new Theme.SagerNet. DarkHighContrast styles (day declares the full dark canvas so it never flashes light; night repeats it as the active forced-night path). - Full M3 role overrides: surface/background/containers, surfaceVariant, outline/outlineVariant, secondary/tertiary, plus status semantics (green/red/yellow) and route/proxy blue. - Generalize the dark-only night-mode handling: selecting any theme in Theme.DARK_ONLY_THEMES (Dracula, Dark High Contrast) forces night mode on and restores the prior setting on exit. Storage key kept for compat. - Surfaced by name in the modern picker list (green preview swatch); excluded from the classic grid. - Picker select() now follows the standard Preference contract (notify-then-persist) so the fragment sees the true previous theme.
Refine the OLED theme to match the intended high-contrast design and fix two issues seen on-device: - App bar / bottom status bar / status & nav bars now use the black surface instead of the green primary, via a new themable appBarBackgroundColor attr (defaults to colorPrimary, so all other themes are unchanged) wired into the shared appbar/toolbar/tab layouts. This reserves green for actions/selection and lets the blue/green/red status text read against black chrome (previously the cyan/blue detail text was washed out on the green bar). - Fix the connect FAB airplane vanishing when connected: the green connected-icon tint was invisible on the green FAB. Add a dedicated fabConnectedColor attr (defaults to statusConnectedColor for back-compat) and set it to the dark on-primary color for Dark High Contrast so the icon stays visible, matching the stopped state. - Black DHC bottom-bar style, dark ActionBar overlay, green tab selection/indicator on the OLED canvas. Verified on Android across stopped/connected states.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/io/nekohasekai/sagernet/ui/SettingsPreferenceFragment.kt`:
- Around line 74-82: The variable previousTheme is assigned from
DataStore.appTheme on line 74, but because the preference change has already
been persisted before this listener is called (persist-then-notify contract),
DataStore.appTheme already contains the new themeId value, not the old one. This
causes previousTheme to equal themeId, making the enteringDarkOnly and
leavingDarkOnly detection incorrect and potentially skipping both force and
restore branches. Capture the actual previous theme value before the preference
gets persisted, possibly by storing it in a field before the preference change
occurs or by obtaining it through a mechanism that provides the prior value
before the persistence happens.
In `@app/src/main/res/values/themes.xml`:
- Around line 881-918: The Theme.SagerNet.Dialog.DarkHighContrast style is
missing an override for fabConnectedColor, which causes the connected FAB icon
to lose contrast since it uses the same green color as the FAB background. Add a
new item element with name="fabConnectedColor" set to "`@color/dhc_on_primary`"
inside the Theme.SagerNet.Dialog.DarkHighContrast style block, matching the
approach already used in the base Theme.SagerNet.DarkHighContrast theme. Apply
this same change to both the values/themes.xml and values-night/themes.xml
files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c6c8c0d2-26c8-41a1-a29a-2e158345aee0
📒 Files selected for processing (18)
app/src/main/java/io/nekohasekai/sagernet/Constants.ktapp/src/main/java/io/nekohasekai/sagernet/database/DataStore.ktapp/src/main/java/io/nekohasekai/sagernet/ui/SettingsPreferenceFragment.ktapp/src/main/java/io/nekohasekai/sagernet/utils/Theme.ktapp/src/main/java/io/nekohasekai/sagernet/widget/ServiceButton.ktapp/src/main/java/moe/matsuri/nb4a/ui/ThemePickerPreference.ktapp/src/main/res/layout/layout_app_list.xmlapp/src/main/res/layout/layout_appbar.xmlapp/src/main/res/layout/layout_apps.xmlapp/src/main/res/layout/layout_group_list.xmlapp/src/main/res/layout/layout_main.xmlapp/src/main/res/layout/layout_tools.xmlapp/src/main/res/layout/layout_webdav_settings.xmlapp/src/main/res/values-night/themes.xmlapp/src/main/res/values/attrs.xmlapp/src/main/res/values/colors.xmlapp/src/main/res/values/strings.xmlapp/src/main/res/values/themes.xml
✅ Files skipped from review due to trivial changes (9)
- app/src/main/res/layout/layout_appbar.xml
- app/src/main/java/io/nekohasekai/sagernet/Constants.kt
- app/src/main/res/layout/layout_group_list.xml
- app/src/main/res/layout/layout_apps.xml
- app/src/main/java/io/nekohasekai/sagernet/database/DataStore.kt
- app/src/main/res/layout/layout_main.xml
- app/src/main/res/layout/layout_webdav_settings.xml
- app/src/main/res/layout/layout_app_list.xml
- app/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/moe/matsuri/nb4a/ui/ThemePickerPreference.kt
Five new full-fledged M3 dark themes surfaced by name in the modern picker (legacy Dracula id 23 left untouched): - Dracula-M3 (id 26), Nord (27), Monokai (28), Ayu (29), Catppuccin (30). - Each: official palette in colors.xml + day/night styles with the full dark canvas, M3 role overrides, black chrome (surface app/bottom bars, dark ActionBar overlay, themed tabs), status semantics, and a visible connected FAB icon (fabConnectedColor = on-primary). Registered in Theme.kt (ids, style maps, MODERN_THEMES, DARK_ONLY_THEMES) + strings. Fix app-bar inflation crash: appBarBackgroundColor must resolve to a CONCRETE @color, not an attr reference. android:background reads it as a drawable via getDrawableForDensity, which does not follow attr-to-attr indirection (?attr -> ?attr crashed AppBarLayout/BottomAppBar inflation). Each theme now sets appBarBackgroundColor to a concrete color matching its colorPrimary (legacy) or dark canvas (modern). StatsBar drops its explicit backgroundTint and inherits the per-theme bottomAppBarStyle instead. Verified on Android: all 6 modern themes apply with correct accents, dark chrome, visible FAB icon, no crashes; legacy themes unchanged.
…olor - Correct Catppuccin Mocha palette to official values: surface = mantle (#181825), surfaceVariant = surface0 (#313244), and primary_dark to a real palette color (sapphire #74C7EC) instead of a custom mauve tint. - Add the missing fabConnectedColor to the Dark High Contrast dialog styles (day + night) for consistency with the app variants and the other modern dialog themes. (CodeRabbit's 'missing theme styles' findings were false positives \u2014 all 12 styles exist and were verified building and on-device.)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/res/values/themes.xml`:
- Around line 258-273: The BottomAppBar variant styles
(Widget.SagerNet.BottomAppBar.DraculaM3, Widget.SagerNet.BottomAppBar.Nord,
Widget.SagerNet.BottomAppBar.Monokai, Widget.SagerNet.BottomAppBar.Ayu, and
Widget.SagerNet.BottomAppBar.Catppuccin) are missing parent attribute
declarations. Add parent="Widget.SagerNet.BottomAppBar" to each of these style
definitions so they inherit the shared base widget configuration (alignment,
cradle, elevation) while only overriding the backgroundTint attribute.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 649f5d91-b6c9-41bf-97ae-2331d274b54a
📒 Files selected for processing (7)
app/src/main/java/io/nekohasekai/sagernet/utils/Theme.ktapp/src/main/res/layout/layout_main.xmlapp/src/main/res/values-night/themes.xmlapp/src/main/res/values/attrs.xmlapp/src/main/res/values/colors.xmlapp/src/main/res/values/strings.xmlapp/src/main/res/values/themes.xml
💤 Files with no reviewable changes (1)
- app/src/main/res/layout/layout_main.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/res/values/attrs.xml
- app/src/main/res/values-night/themes.xml
Address review feedback: - Dracula-M3: profile names now use Dracula yellow (#F1FA8C) instead of white, matching the authentic Dracula look. - Dark High Contrast: the bottom-bar speed arrows and the 'tap to check connection' detail text now use a muted near-white (on-surface-variant) instead of bright blue \u2014 a more restrained OLED palette in the spirit of F-Droid's true-black theme; blue stays reserved for links/route-proxy. - Lift the bottom status bar (and the docked FAB + its anchored progress ring) above the navigation-bar inset under edge-to-edge, so the loading ring is no longer clipped by the system inset. Verified on Android (Dracula-M3 + Dark High Contrast).
Give the modern themes' Widget.SagerNet.BottomAppBar.* variants an explicit parent=Widget.SagerNet.BottomAppBar so they keep the shared cradle/alignment/elevation config and only override backgroundTint, rather than relying on implicit dotted-name inheritance.
|
Re: the |
Address review feedback: - Dark High Contrast: use green (colorPrimary) for colorAccent/colorSecondary so section headers (App Settings / Route Settings) and switch toggles are green, matching the green identity (and the theme's green picker swatch), instead of the previously confusing blue. Blue (colorTertiary / route proxy / links) stays reserved for links. - Rename the picker's 'Classic colors...' entry to 'Legacy colors...'. - Add top padding + spacing above the connection-test progress dialog's loading circle so it isn't flush against the dialog's top edge. Verified on Android (DHC: green header + green toggle).
Replace the last non-M3 themed widgets so the app is fully Material 3: - SwitchCompat -> MaterialSwitch in layout_route_item and layout_apps_item. - CheckBox -> MaterialCheckBox in layout_backup and layout_import (6 total). - androidx.appcompat.widget.Toolbar -> MaterialToolbar in webdav settings. All are API-compatible subclasses, so ViewBinding/findViewById and the existing isChecked/listener calls are unchanged. Only AppCompatImageView remains, which is the idiomatic M3 image view (not an M2 themed widget). Verified on Android: route/apps MaterialSwitch toggles render M3 (green on Dark High Contrast), no crashes.
- Theme picker: Dark High Contrast preview swatch is now a black circle with a white circumference ring (via ThemeInfo.ringColor + a GradientDrawable oval) so it reads as 'OLED black', not 'a green theme'. Other modern themes keep their solid accent dot. - Profile + route lists: increase bottom padding to 96dp (with clipToPadding=false) so the last card clears the docked FAB and the bottom stats bar instead of being hidden behind them when scrolled to the bottom. Verified on Android (DHC swatch ring; last card clears FAB at scroll end).
96dp wasn't enough: when connected, the pinned stats bar (~115dp) plus the FAB cradle overhang (~40dp) overlapped the last card. Measured the FAB top sitting ~431px above the screen bottom on the test device, so bump the profile + route list bottom padding to 150dp (with clipToPadding=false) so the last card always clears the FAB and stats bar in both connected and disconnected states. Verified on Android: last card fully visible above the FAB at scroll end.
- Add a RecyclerView scroll listener on the profile list: hide the docked FAB on downward scroll, show it on upward scroll or when the list settles (idle). Only acts in stable Stopped/Connected states so it doesn't fight the FAB's connect/disconnect animation. - Bump profile + route list bottom padding to 168dp (FAB occupies ~157dp at 440dpi) so the last card always rests above the docked FAB and the pinned stats bar, in both connected and disconnected states. Verified on Android (Dracula-M3, connected): last card clears the FAB at the scroll extent; FAB tucks away while scrolling.
Summary
Adds a modern theme system on top of the Material 3 migration: a two-tier picker and six full-fledged M3 dark themes, surfaced by name.
Phase 1 — Two-tier theme picker
ThemePickerPreferenceopens a modern named list of M3 themes (preview swatch + name), with a trailing "Classic colors…" row that opens the original swatch grid for the legacy single-accent palettes.Theme.MODERN_THEMESis a declarative catalog; the classic grid skips any id in the modern list (no duplication). Removes the supersededColorPickerPreference.appThemeint key.Phase 2 — Dark High Contrast (OLED)
Theme.DARK_ONLY_THEMES(forces night on, restores prior setting on exit; storage key kept for back-compat).fabConnectedColorattr fixes the connect FAB airplane vanishing when connected (green icon on a green FAB) — icon stays visible.Phase 3 — More M3 dark themes
appBarBackgroundColorresolves to a concrete @color per theme (android:background can't follow attr-to-attr indirection).Testing
Greptile Summary
This PR adds a two-tier theme picker and six full Material 3 dark themes (Dark High Contrast, Dracula M3, Nord, Monokai, Ayu, Catppuccin) on top of the existing M3 migration, while also fixing the connected-FAB icon visibility and the
AppBarLayoutattr-indirection crash.ThemePickerPreference): a modern named list (swatch + label, ScrollView-wrapped) with a "Classic colors…" row that opens the original swatch grid;ColorPickerPreferenceremoved.DARK_ONLY_THEMES;nightThemeBeforeDraculastore key kept for backward compatibility.CheckBox→MaterialCheckBox,SwitchCompat→MaterialSwitch,appBarBackgroundColorattr replaces?colorPrimaryon every toolbar/tab strip (fixing the inflation crash), and a FAB scroll-hide listener is added toConfigurationFragment.Confidence Score: 4/5
Safe to merge with one logic fix: the night-mode coercion in SettingsPreferenceFragment silently no-ops when switching between two dark-only themes while night mode is not active.
The night-mode guard on line 82 uses !leavingDarkOnly as a combined condition that both skips saving the restore value and skips forcing night on. When the user is on a dark-only theme, manually overrides night mode to light, and then switches to another dark-only theme, night mode is never re-enabled — leaving the new theme's dark canvas inactive. All other changes (theme XML, picker UI, FAB tint, layout migrations) look correct.
app/src/main/java/io/nekohasekai/sagernet/ui/SettingsPreferenceFragment.kt — the enteringDarkOnly branch (lines 81–90)
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[User selects theme] --> B{enteringDarkOnly?} B -- No --> C{leavingDarkOnly?} C -- No --> Z[Apply theme, recreate activity] C -- Yes --> D[Restore nightThemeBeforeDracula if != -1] D --> Z B -- Yes --> E{!leavingDarkOnly AND nightTheme != 1?} E -- Yes --> F[Save nightThemeBeforeDracula / Force nightTheme=1 / applyNightTheme] F --> Z E -- No --> G[No-op: night mode not forced on] G --> Z style G fill:#ff9999,stroke:#cc0000%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[User selects theme] --> B{enteringDarkOnly?} B -- No --> C{leavingDarkOnly?} C -- No --> Z[Apply theme, recreate activity] C -- Yes --> D[Restore nightThemeBeforeDracula if != -1] D --> Z B -- Yes --> E{!leavingDarkOnly AND nightTheme != 1?} E -- Yes --> F[Save nightThemeBeforeDracula / Force nightTheme=1 / applyNightTheme] F --> Z E -- No --> G[No-op: night mode not forced on] G --> Z style G fill:#ff9999,stroke:#cc0000Reviews (8): Last reviewed commit: "Hide FAB on scroll + 168dp list padding ..." | Re-trigger Greptile